Repository navigation
Conversation
… net.Server tests Both files wrapped the expected object in expect.objectContaining() and gave the matcher to toMatchObject. toMatchObject walks the enumerable keys of the expected value, and a matcher has none, so nothing was compared. Three expected values of url-parse-ipv6.test.ts were wrong. They now hold what url.parse returns in Node 26.3.0 and in Bun.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 2 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
|
Status: ready for review. Test-only. Reproduced on a release build of main (271b65e): with CI (build 136856): the two test files of this PR pass on every lane. Each red test is under |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — test-only change that turns a vacuous toMatchObject(expect.objectContaining(...)) into a live assertion and corrects the three rows that had been silently wrong.
What was reviewed:
- Traced the three corrected rows through
src/js/node/url.ts: port is appended tohost/href(lines 411-414), the hostname is lowercased before the IPv6 branch (line 383), and a protocol-less bracketed input never enters the host branch, consistent with the already-live[::1]row in thedescribe.eachblock. - Checked the sibling rows that are now live: the trailing-colon row matches
parseHost(port":"is dropped), and theurl.parse(input, true)form setssearch: nullwith a null-prototypequerywhen there is no query string, matching Node. - Confirmed all six
net.Server.prototypemethods are own enumerableServer.prototype.X = functionassignments insrc/js/node/net.ts, so the now-stricttoMatchObjectcannot fail spuriously.
Extended reasoning...
The diff touches only two test files (test/js/node/url/url-parse-ipv6.test.ts and test/js/node/net/server.spec.ts), removing an expect.objectContaining wrapper from toMatchObject at four call sites and correcting three expected rows in the IPv6 spot-check table; no runtime code changes. It touches no security-sensitive surface. Approval was decided by the change being small and mechanical, by every corrected expectation and every newly live sibling row tracing directly to Bun's url.parse and net.Server implementations (which mirror Node's), and by the changed files not being covered by CODEOWNERS; the bug hunt exited on dry_streak with no findings.
Problem
test/js/node/url/url-parse-ipv6.test.tsandtest/js/node/net/server.spec.tsgiveexpect.objectContaining(expected)totoMatchObject. Inbun testthat compares nothing:toMatchObjectwalks the enumerable keys of the expected value, and a matcher has none (bun:test: apply an asymmetric matcher that is the whole expected value of toMatchObject and of snapshot property matchers #37912).url-parse-ipv6.test.tspassed.Fix
toMatchObjectdirectly, in both files.url.parsereturns in Node 26.3.0 and in Bun:http://[::1]:1keeps its port inhostandhref,http://[::F]has the host[::f], and[0:0:0:1:2:3:4:5]has no protocol, so it is a path andhostisnull.url-parse-ipv6.test.tsfail. With the corrected rows, both files pass (74 pass, 3 skip).Background
url.parsealready returns Node's values for these inputs.server.spec.ts("has the expected methods") holds without the wrapper, so only its form changes.[auto-merge] gate passed · iteration 0 · 2 files touched
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file